feat(baseball-scoreboard): team-color grid, clearer at-bat labels, player card (v1.19.0)#180
Conversation
…ayer card (v1.19.0)
Three enhancements to the baseball plugin:
1. Traditional scoreboard: subtle team-color row tint (~12% brightness
wash) plus a solid left-edge accent strip per team, reusing the
already-fetched away/home ESPN colors. Gated by a new
show_team_color_backgrounds toggle (default on; requires
use_team_colors). Text stays legible via its black outline.
2. Pitcher/Batter screen: labels spelled out ("Pitcher:" / "Batter:")
so "B" is no longer ambiguous with the grid's Balls indicator.
3. New Player Card screen (show_player_card, MLB & NCAA): rotates in a
masters-style card for the current batter/pitcher with headshot,
jersey number, position, bat/throw, and season stats (AVG/HR/RBI for
hitters, ERA/W-L/K for pitchers). Bio + headshot fetched from ESPN's
athlete API and cached (memory + disk); the headshot hides on tiny
panels and the card degrades to text-only when unavailable. Skipped
for MiLB (no ESPN player data).
All new code lives in existing files (no new bare-name modules, per the
cross-plugin collision rule). Adds config schema (grid toggle, per-league
show_player_card, player_card customization block), manager plumbing,
docs, and tests (test_player_card.py + grid-tint tests).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R1z28MrGx56x6eiJvibDg9
📝 WalkthroughWalkthroughThis PR adds a full-screen Player Card display with ESPN player bios, stats, and cached headshots. It also adds team-color row tinting, configuration and manager wiring, roster enrichment, regression tests, documentation, and version metadata updates. ChangesBaseball Scoreboard Player Card and Team-Color Tint
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant BaseballLive
participant ESPNDataSource
participant BaseballLogoManager
participant Display
BaseballLive->>BaseballLive: extract pitcher and batter IDs
BaseballLive->>ESPNDataSource: fetch player bio
ESPNDataSource-->>BaseballLive: return normalized bio
BaseballLive->>BaseballLogoManager: load headshot
BaseballLogoManager-->>BaseballLive: return headshot or None
BaseballLive->>Display: draw player card
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 309 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Address a Codacy medium security finding on the new player-card headshot loader: - Sanitize the ESPN-provided player_id and league before using them as filesystem path segments (_safe_filename strips to [A-Za-z0-9_-]) so an unexpected id can't escape the headshot cache directory via '..' or path separators. - Only fetch headshots over http(s), refusing file://, ftp://, etc. Adds a unit test for the filename sanitization. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1z28MrGx56x6eiJvibDg9
…nt pass Resolve the Codacy/Bandit B110 (try/except/pass) medium finding in _fetch_player_bio: the two cache_manager get/set guards swallowed exceptions with a bare `pass`. Log them at debug level instead so cache issues are diagnosable, matching how the rest of the plugin handles non-fatal cache errors. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1z28MrGx56x6eiJvibDg9
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/baseball-scoreboard/logo_manager.py`:
- Around line 241-260: The headshot download in the URL-fetching block lacks
host validation and a response-size limit. Before requests.get, parse the URL
and allow only expected ESPN CDN hostnames, rejecting all others; then enforce a
bounded response size using Content-Length and/or limited resp.iter_content
accumulation before Image.open. Preserve the existing timeout, caching, logging,
and _crop_square behavior in the surrounding headshot-loading logic.
- Around line 212-262: Move headshot HTTP fetching out of the synchronous render
path: update BaseballLive.update() to prefetch/cache player headshots using the
existing throttled background-thread and queue pattern from _fetch_player_bio,
with a bounded wait and failure handling. Change logo_manager.load_headshot() to
serve only memory/disk-cached images during rendering, and ensure
_draw_player_card_screen never invokes requests.get indirectly on a cache miss.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02f95740-78d4-4f41-8a64-801aca6b0afd
📒 Files selected for processing (12)
plugins.jsonplugins/baseball-scoreboard/.gitignoreplugins/baseball-scoreboard/CHANGELOG.mdplugins/baseball-scoreboard/README.mdplugins/baseball-scoreboard/baseball.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/data_sources.pyplugins/baseball-scoreboard/logo_manager.pyplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/test_player_card.pyplugins/baseball-scoreboard/test_traditional_scoreboard.py
…t review) Address two CodeRabbit review findings on the player-card headshot loader: 1. SSRF + unbounded response (security): the ESPN-supplied headshot URL was fetched with only a scheme check. Now restrict the host to ESPN domains (*.espncdn.com / *.espn.com) via an allowlist, and stream the download with a hard 5 MB size cap (Content-Length check + bounded iter_content) to prevent memory exhaustion. 2. Render-path blocking (stability): load_headshot() previously did a synchronous requests.get on a cache miss straight from _draw_player_card_screen, freezing the display up to 5s when a new batter/pitcher rotated in. The render path is now cache-only (allow_download=False); BaseballLive.update() prefetches headshot bytes off-thread (daemon thread, throttled) to warm the disk cache, mirroring the existing _fetch_player_bio pattern. A cold cache degrades to a text-only card until the prefetch completes. Adds tests for the host allowlist (incl. metadata-IP and look-alike-host rejection) and for the render path never downloading. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1z28MrGx56x6eiJvibDg9
Resolve plugins.json conflict (last_updated timestamp only) by taking main's later date and re-running update_registry.py to confirm the whole file is consistent with all manifests post-merge.
Same recurring issue as PRs #175/#177/#178: the core's store_manager.py validator flags ledmatrix_min as deprecated and expects ledmatrix_min_version. This PR's new 1.19.0 changelog entry used the deprecated name; corrected to match the non-deprecated field (older entries below 1.6.2 already have this same pre-existing issue but are untouched by this PR, so left as-is -- out of scope here).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/baseball-scoreboard/test_player_card.py (1)
205-229: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a test for the 5 MB download size cap.
The PR enforces a 5 MB response limit in
_download_headshot_image, but no test verifies that oversized responses are rejected. The existing tests cover URL allowlisting and render-path no-download behavior, but the size cap is a security control that should be validated. A mock-based test patchinglm.requests.getto return a response exceeding 5 MB would close this gap without network calls.♻️ Suggested test for the 5 MB download cap
+def test_download_headshot_rejects_oversized_response(): + import logging + import logo_manager as lm + + class FakeResponse: + def __init__(self, data): + self._data = data + self.headers = {"content-length": str(len(data))} + def iter_content(self, chunk_size=8192): yield self._data + def close(self): + pass + + mgr = lm.BaseballLogoManager(None, logging.getLogger("t")) + orig_get = lm.requests.get + oversized = b"\x00" * (5 * 1024 * 1024 + 1) + + def fake_get(*a, **k): + return FakeResponse(oversized) + + lm.requests.get = fake_get + try: + result = mgr.load_headshot( + "oversized_id", "https://a.espncdn.com/x.png", + league="mlb", max_size=32, allow_download=True, + ) + assert result is None, "Oversized headshot should be rejected" + finally: + lm.requests.get = orig_get + print("test_download_headshot_rejects_oversized_response: PASS")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/baseball-scoreboard/test_player_card.py` around lines 205 - 229, Add a mock-based test alongside test_load_headshot_render_path_never_downloads that patches lm.requests.get with an oversized response exceeding 5 MB, invokes BaseballLogoManager.load_headshot with an allowed ESPN URL and allow_download=True, and asserts the result is None. Restore the original requests.get in a finally block and verify the mocked response is used without real network access.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@plugins/baseball-scoreboard/test_player_card.py`:
- Around line 205-229: Add a mock-based test alongside
test_load_headshot_render_path_never_downloads that patches lm.requests.get with
an oversized response exceeding 5 MB, invokes BaseballLogoManager.load_headshot
with an allowed ESPN URL and allow_download=True, and asserts the result is
None. Restore the original requests.get in a finally block and verify the mocked
response is used without real network access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b1faf1c-6868-4dca-8b0d-db317386cce6
📒 Files selected for processing (5)
plugins.jsonplugins/baseball-scoreboard/baseball.pyplugins/baseball-scoreboard/logo_manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/test_player_card.py
✅ Files skipped from review due to trivial changes (2)
- plugins.json
- plugins/baseball-scoreboard/manifest.json
Pull Request
Summary
Three aesthetic/usability enhancements to the baseball plugin: a subtle team-color tint on the traditional grid scoreboard, spelled-out "Pitcher:" / "Batter:" labels (so "B" is no longer ambiguous with the grid's Balls indicator), and a new masters-style player card with headshot, jersey number, position, bat/throw, and season stats for the current batter/pitcher.
Type of change
Plugin(s) affected
baseball-scoreboardRelated issues
N/A
Test plan
EMULATOR=true python3 run.py)scripts/dev_server.py)Details:
test_player_card.py(17 tests): roster info-map join, current-batter/pitcher ID scan, ESPN bio parsing (_parse_player_details), stat selection (hitter vs pitcher vs generic fallback), team-color resolution, gatekeeper behavior (MiLB skip, skip-when-no-bio, favorites-only), and a render smoke test across all four panel sizes (64×32, 128×32, 128×64, 256×32) asserting no margin overflow and headshot-hidden on the tiny tier.test_traditional_scoreboard.py(tint present when enabled, absent when disabled while solid team colors still render).test_baseball_plugin, onetest_config_reloadassertion,test_test_mode_live_games) are pre-existing and reproduce identically onmain; they require the coresrc/fonts tree.config_schema.jsonandmanifest.jsonvalidate as JSON;update_registry.pysyncedplugins.jsonto 1.19.0;scripts/check_module_collisions.pypasses (no new bare-name modules).Required for plugin changes
versioninplugins/<id>/manifest.json(1.18.0 → 1.19.0)class_nameinmanifest.jsonmatches the actual class inmanager.pyexactlyentry_pointmatches the real file (or is omitted to use themanager.pydefault)README.md(documented the grid toggle, label change, and new Player Card screen + options)config_schema.jsonis the source of truth for the web UI form — new options added withdefault,description, and constraints (show_team_color_backgrounds, per-leagueshow_player_card, and aplayer_cardcustomization block)plugins.json) — hook not installed in this environment; ranupdate_registry.pymanually instead and committed the syncedplugins.jsonChecklist
CONTRIBUTING.mdCONTRIBUTING.mdandCODE_OF_CONDUCT.mdNotes for reviewer
baseball.py,data_sources.py,logo_manager.py) to avoid the cross-plugin bare-name collision rule inCLAUDE.md.away_team_color/home_team_color. Jersey/position/headshot-URL come free from the ESPN game-summary rosters; only the season stats need the extraathletes/{id}+/overviewfetch (mirrors the masters plugin), cached in memory and on disk (assets/headshots/, gitignored).🤖 Generated with Claude Code
https://claude.ai/code/session_01R1z28MrGx56x6eiJvibDg9
Generated by Claude Code
Summary by CodeRabbit